Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Limit the number of names and values in check-cfg diagnostics #121202

Merged
merged 1 commit into from
Mar 6, 2024

Conversation

Urgau
Copy link
Member

@Urgau Urgau commented Feb 16, 2024

The Rust for Linux feedback to the check-cfg Call for Testing, revealed a weakness in the check-cfg. They are unbounded and in the case RfL they have ~20k cfgs and having them printed (even once) is unbearable.

This PR limits it to 35 (28 rustc well known + feature + docsrs + 5 custom) which feels like a good middle ground for regular users (i.e. Cargo users).

When it goes over that limit print the N first with " and X more".

@rustbot label +F-check-cfg

@rustbot
Copy link
Collaborator

rustbot commented Feb 16, 2024

r? @pnkfelix

rustbot has assigned @pnkfelix.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Feb 16, 2024
@Urgau Urgau force-pushed the check-cfg-limit-diagnostics branch from 88c6f3d to 3b719fc Compare February 16, 2024 22:38
@rustbot rustbot added the F-check-cfg --check-cfg label Feb 16, 2024
@Urgau Urgau force-pushed the check-cfg-limit-diagnostics branch from 3b719fc to 1ed04df Compare February 16, 2024 23:31
@juntyr
Copy link
Contributor

juntyr commented Feb 17, 2024

Are there cases where it might be necessary to read the full list? In that case, perhaps it could be written to a file on disk and the error could give that file path, similar to how very long types are written to disk.

@Urgau
Copy link
Member Author

Urgau commented Feb 17, 2024

I proposed it in the issue and the responses were no.

@ojeda:

Yeah, that's not fine. I will send a fix for it, probably similar to our very long types where we write the full type to disk.

Thanks! However, I wouldn't write the list to disk -- I don't think it is useful to see the list, and we would need to ignore those files in .gitignore and so on.

@epage:

Maybe its just me but I feel like if the list of suggestions is too long, its just not worth doing anything with. I don't see it being likely someone is going to open a file with 29k options to find the exact one they wanted.

@bors
Copy link
Contributor

bors commented Feb 25, 2024

☔ The latest upstream changes (presumably #121591) made this pull request unmergeable. Please resolve the merge conflicts.

@Urgau Urgau force-pushed the check-cfg-limit-diagnostics branch from 1ed04df to 10d5089 Compare February 25, 2024 21:32
@pnkfelix
Copy link
Member

I will r+ this once we resolve the question of whether to use an environment variable or a -Z-flag to restore the prior behavior.

@rustbot label: -S-waiting-on-review +S-waiting-on-author

@rustbot rustbot added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Feb 27, 2024
@bors
Copy link
Contributor

bors commented Feb 28, 2024

☔ The latest upstream changes (presumably #121489) made this pull request unmergeable. Please resolve the merge conflicts.

@pnkfelix
Copy link
Member

The team prefers -Z flags over environment variables for these sorts of things

@rustbot author

@rustbot rustbot added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Feb 29, 2024
@Urgau Urgau force-pushed the check-cfg-limit-diagnostics branch from 10d5089 to 91322f4 Compare February 29, 2024 20:55
@Urgau
Copy link
Member Author

Urgau commented Feb 29, 2024

Flag added under -Zcheck-cfg-all-expected.

@rustbot ready

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Feb 29, 2024
@pnkfelix
Copy link
Member

pnkfelix commented Mar 4, 2024

@bors r+

@bors
Copy link
Contributor

bors commented Mar 4, 2024

📌 Commit 91322f4 has been approved by pnkfelix

It is now in the queue for this repository.

@bors bors removed the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Mar 4, 2024
@bors bors added the S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. label Mar 4, 2024
jhpratt added a commit to jhpratt/rust that referenced this pull request Mar 5, 2024
… r=pnkfelix

Limit the number of names and values in check-cfg diagnostics

The Rust for Linux [feedback](rust-lang#82450 (comment)) to the check-cfg Call for Testing, revealed a weakness in the check-cfg. They are unbounded and in the case RfL they have ~20k cfgs and having them printed (even once) is unbearable.

This PR limits it to 35 (28 rustc well known + `feature` + `docsrs` + 5 custom) which feels like a good middle ground for regular users (i.e. Cargo users).

When it goes over that limit print the N first with " and X more".

``@rustbot`` label +F-check-cfg
bors added a commit to rust-lang-ci/rust that referenced this pull request Mar 5, 2024
Rollup of 15 pull requests

Successful merges:

 - rust-lang#121065 (Add basic i18n guidance for `Display`)
 - rust-lang#121202 (Limit the number of names and values in check-cfg diagnostics)
 - rust-lang#121213 (Add an example to demonstrate how Rc::into_inner works)
 - rust-lang#121262 (Add vector time complexity)
 - rust-lang#121287 (Clarify/add `must_use` message for Rc/Arc/Weak::into_raw.)
 - rust-lang#121664 (Adjust error `yield`/`await` lowering)
 - rust-lang#121838 (Use the correct logic for nested impl trait in assoc types)
 - rust-lang#121860 (Add a tidy check that checks whether the fluent slugs only appear once)
 - rust-lang#121913 (Don't panic when waiting on poisoned queries)
 - rust-lang#121959 (Removing absolute path in proc-macro)
 - rust-lang#121975 (hir_analysis: enums return `None` in `find_field`)
 - rust-lang#121978 (Fix duplicated path in the "not found dylib" error)
 - rust-lang#121987 (pattern analysis: abort on arity mismatch)
 - rust-lang#121993 (Avoid using unnecessary queries when printing the query stack in panics)
 - rust-lang#121997 (interpret/cast: make more matches on FloatTy properly exhaustive)

r? `@ghost`
`@rustbot` modify labels: rollup
@bors
Copy link
Contributor

bors commented Mar 5, 2024

☔ The latest upstream changes (presumably #121780) made this pull request unmergeable. Please resolve the merge conflicts.

@bors bors added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Mar 5, 2024
@Urgau Urgau force-pushed the check-cfg-limit-diagnostics branch from 91322f4 to 9d9b26b Compare March 5, 2024 07:02
@Urgau
Copy link
Member Author

Urgau commented Mar 5, 2024

Rebased.

@rustbot ready

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Mar 5, 2024
@pnkfelix
Copy link
Member

pnkfelix commented Mar 5, 2024

@bors r+

@bors
Copy link
Contributor

bors commented Mar 5, 2024

📌 Commit 9d9b26b has been approved by pnkfelix

It is now in the queue for this repository.

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Mar 5, 2024
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Mar 5, 2024
… r=pnkfelix

Limit the number of names and values in check-cfg diagnostics

The Rust for Linux [feedback](rust-lang#82450 (comment)) to the check-cfg Call for Testing, revealed a weakness in the check-cfg. They are unbounded and in the case RfL they have ~20k cfgs and having them printed (even once) is unbearable.

This PR limits it to 35 (28 rustc well known + `feature` + `docsrs` + 5 custom) which feels like a good middle ground for regular users (i.e. Cargo users).

When it goes over that limit print the N first with " and X more".

`@rustbot` label +F-check-cfg
bors added a commit to rust-lang-ci/rust that referenced this pull request Mar 5, 2024
…iaskrgr

Rollup of 8 pull requests

Successful merges:

 - rust-lang#121202 (Limit the number of names and values in check-cfg diagnostics)
 - rust-lang#121301 (errors: share `SilentEmitter` between rustc and rustfmt)
 - rust-lang#121658 (Hint user to update nightly on ICEs produced from outdated nightly)
 - rust-lang#121846 (only compare ambiguity item that have hard error)
 - rust-lang#121961 (add test for rust-lang#78894 rust-lang#71450)
 - rust-lang#121975 (hir_analysis: enums return `None` in `find_field`)
 - rust-lang#121978 (Fix duplicated path in the "not found dylib" error)
 - rust-lang#121991 (Merge impl_trait_in_assoc_types_defined_by query back into `opaque_types_defined_by`)

r? `@ghost`
`@rustbot` modify labels: rollup
bors added a commit to rust-lang-ci/rust that referenced this pull request Mar 6, 2024
…iaskrgr

Rollup of 8 pull requests

Successful merges:

 - rust-lang#121202 (Limit the number of names and values in check-cfg diagnostics)
 - rust-lang#121301 (errors: share `SilentEmitter` between rustc and rustfmt)
 - rust-lang#121658 (Hint user to update nightly on ICEs produced from outdated nightly)
 - rust-lang#121846 (only compare ambiguity item that have hard error)
 - rust-lang#121961 (add test for rust-lang#78894 rust-lang#71450)
 - rust-lang#121975 (hir_analysis: enums return `None` in `find_field`)
 - rust-lang#121978 (Fix duplicated path in the "not found dylib" error)
 - rust-lang#121991 (Merge impl_trait_in_assoc_types_defined_by query back into `opaque_types_defined_by`)

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit 640648b into rust-lang:master Mar 6, 2024
11 checks passed
@rustbot rustbot added this to the 1.78.0 milestone Mar 6, 2024
rust-timer added a commit to rust-lang-ci/rust that referenced this pull request Mar 6, 2024
Rollup merge of rust-lang#121202 - Urgau:check-cfg-limit-diagnostics, r=pnkfelix

Limit the number of names and values in check-cfg diagnostics

The Rust for Linux [feedback](rust-lang#82450 (comment)) to the check-cfg Call for Testing, revealed a weakness in the check-cfg. They are unbounded and in the case RfL they have ~20k cfgs and having them printed (even once) is unbearable.

This PR limits it to 35 (28 rustc well known + `feature` + `docsrs` + 5 custom) which feels like a good middle ground for regular users (i.e. Cargo users).

When it goes over that limit print the N first with " and X more".

``@rustbot`` label +F-check-cfg
ojeda added a commit to ojeda/linux that referenced this pull request Apr 1, 2024
This is the next upgrade to the Rust toolchain, from 1.77.1 to 1.78.0
(i.e. the latest) [1].

See the upgrade policy [2] and the comments on the first upgrade in
commit 3ed03f4 ("rust: upgrade to Rust 1.68.2").

# Unstable features

There have been no changes to the set of unstable features used in
our own code. Therefore, the only unstable features allowed to be used
outside the `kernel` crate is still `new_uninit`.

However, since we are finally dropping our `alloc` fork [3], all the
unstable features used by `alloc` (~30 language ones, ~60 library ones)
are not a concern anymore. This reduces the maintanance burden, increases
the chances of new compiler versions working without changes and gets
us closer to the goal of supporting several compiler versions.

It also means that, ignoring non-language/library features, we are
currently left with just the few language features needed to implement the
kernel `Arc`, the `new_uninit` library feature, the `compiler_builtins`
marker and the few `no_*` `cfg`s we pass when compiling `core`/`alloc`.

Please see [4] for details.

# Required changes

## LLVM's data layout

Rust 1.77.0 (i.e. the previous upgrade) introduced a check for matching
LLVM data layouts [5]. Then, Rust 1.78.0 upgraded LLVM's bundled major
version from 17 to 18 [6], which changed the data layout in x86 [7]. Thus
update the data layout in our custom target specification for x86 so
that the compiler does not complain about the mismatch:

    error: data-layout for target `target-5559158138856098584`,
    `e-m:e-p270:32:32-p271:32:32-p272:64:64-i64:64-f80:128-n8:16:32:64-S128`,
    differs from LLVM target's `x86_64-linux-gnu` default layout,
    `e-m:e-p270:32:32-p271:32:32-p272:64:64-i64:64-i128:128-f80:128-n8:16:32:64-S128`

In the future, the goal is to drop the custom target specification
files. Meanwhile, if we want to support other LLVM versions used
in `rustc` (e.g. for LTO), we will need to add some extra logic
(e.g. conditional on LLVM's version, or extracting the data layout from
an existing built-in target specification).

## `unused_imports`

Rust's `unused_imports` lint covers both unused and redudant imports.
Now, in 1.78.0, the lint detects more cases of redundant imports [8].
Thus the previous commit cleaned them up.

## Clippy's `new_without_default`

Clippy now suggests to implement `Default` even when `new()` is `const`,
since `Default::default()` may call `const` functions even if it is not
`const` itself [9]. Thus the previous commit added the implementation.

# Other changes in Rust

Rust 1.78.0 introduces `feature(asm_goto)` [10] [11]. This feature was
discussed in the past [12].

Rust 1.78.0 introduced support for mutable pointers to Rust statics,
including a test case for the Linux kernel's `VTABLE` use case [13].

Rust 1.78.0 with debug assertions enabled (i.e. `-Cdebug-assertions=y`,
kernel's `CONFIG_RUST_DEBUG_ASSERTIONS=y`) will now always check all
unsafe preconditions, without a way to opt-out for particular cases [14].

Rust 1.78.0 also improved a couple issues we reported when giving feedback
for the new `--check-cfg` feature [15] [16].

# `alloc` upgrade and reviewing

As mentioned above, compiler upgrades will not update `alloc` anymore,
since we are dropping our `alloc` fork [3].

As a bonus, even if that series is not applied, the new compiler release
happens to build cleanly the existing `alloc` too (i.e. the previous
version's).

Link: https://github.com/rust-lang/rust/blob/stable/RELEASES.md#version-1770-2024-03-21 [1]
Link: https://rust-for-linux.com/rust-version-policy [2]
Link: https://lore.kernel.org/rust-for-linux/20240328013603.206764-1-wedsonaf@gmail.com/ [3]
Link: Rust-for-Linux#2 [4]
Link: rust-lang/rust#120062 [5]
Link: rust-lang/rust#120055 [6]
Link: https://reviews.llvm.org/D86310 [7]
Link: rust-lang/rust#117772 [8]
Link: rust-lang/rust-clippy#10903 [9]
Link: rust-lang/rust#119365 [10]
Link: rust-lang/rust#119364 [11]
Link: https://lore.kernel.org/rust-for-linux/ZWipTZysC2YL7qsq@Boquns-Mac-mini.home/ [12]
Link: rust-lang/rust#120932 [13]
Link: rust-lang/rust#120969 [14]
Link: rust-lang/rust#121202 [15]
Link: rust-lang/rust#121237 [16]
Signed-off-by: Miguel Ojeda <ojeda@kernel.org>
ojeda added a commit to ojeda/linux that referenced this pull request Apr 1, 2024
This is the next upgrade to the Rust toolchain, from 1.77.1 to 1.78.0
(i.e. the latest) [1].

See the upgrade policy [2] and the comments on the first upgrade in
commit 3ed03f4 ("rust: upgrade to Rust 1.68.2").

# Unstable features

There have been no changes to the set of unstable features used in
our own code. Therefore, the only unstable features allowed to be used
outside the `kernel` crate is still `new_uninit`.

However, since we are finally dropping our `alloc` fork [3], all the
unstable features used by `alloc` (~30 language ones, ~60 library ones)
are not a concern anymore. This reduces the maintenance burden, increases
the chances of new compiler versions working without changes and gets
us closer to the goal of supporting several compiler versions.

It also means that, ignoring non-language/library features, we are
currently left with just the few language features needed to implement the
kernel `Arc`, the `new_uninit` library feature, the `compiler_builtins`
marker and the few `no_*` `cfg`s we pass when compiling `core`/`alloc`.

Please see [4] for details.

# Required changes

## LLVM's data layout

Rust 1.77.0 (i.e. the previous upgrade) introduced a check for matching
LLVM data layouts [5]. Then, Rust 1.78.0 upgraded LLVM's bundled major
version from 17 to 18 [6], which changed the data layout in x86 [7]. Thus
update the data layout in our custom target specification for x86 so
that the compiler does not complain about the mismatch:

    error: data-layout for target `target-5559158138856098584`,
    `e-m:e-p270:32:32-p271:32:32-p272:64:64-i64:64-f80:128-n8:16:32:64-S128`,
    differs from LLVM target's `x86_64-linux-gnu` default layout,
    `e-m:e-p270:32:32-p271:32:32-p272:64:64-i64:64-i128:128-f80:128-n8:16:32:64-S128`

In the future, the goal is to drop the custom target specification
files. Meanwhile, if we want to support other LLVM versions used
in `rustc` (e.g. for LTO), we will need to add some extra logic
(e.g. conditional on LLVM's version, or extracting the data layout from
an existing built-in target specification).

## `unused_imports`

Rust's `unused_imports` lint covers both unused and redundant imports.
Now, in 1.78.0, the lint detects more cases of redundant imports [8].
Thus one of the previous patches cleaned them up.

## Clippy's `new_without_default`

Clippy now suggests to implement `Default` even when `new()` is `const`,
since `Default::default()` may call `const` functions even if it is not
`const` itself [9]. Thus one of the previous patches implemented it.

# Other changes in Rust

Rust 1.78.0 introduced `feature(asm_goto)` [10] [11]. This feature was
discussed in the past [12].

Rust 1.78.0 introduced support for mutable pointers to Rust statics,
including a test case for the Linux kernel's `VTABLE` use case [13].

Rust 1.78.0 with debug assertions enabled (i.e. `-Cdebug-assertions=y`,
kernel's `CONFIG_RUST_DEBUG_ASSERTIONS=y`) now always checks all unsafe
preconditions, without a way to opt-out for particular cases [14].

Rust 1.78.0 also improved a couple issues we reported when giving feedback
for the new `--check-cfg` feature [15] [16].

# `alloc` upgrade and reviewing

As mentioned above, compiler upgrades will not update `alloc` anymore,
since we are dropping our `alloc` fork [3].

Link: https://github.com/rust-lang/rust/blob/stable/RELEASES.md#version-1780-2024-05-02 [1]
Link: https://rust-for-linux.com/rust-version-policy [2]
Link: https://lore.kernel.org/rust-for-linux/20240328013603.206764-1-wedsonaf@gmail.com/ [3]
Link: Rust-for-Linux#2 [4]
Link: rust-lang/rust#120062 [5]
Link: rust-lang/rust#120055 [6]
Link: https://reviews.llvm.org/D86310 [7]
Link: rust-lang/rust#117772 [8]
Link: rust-lang/rust-clippy#10903 [9]
Link: rust-lang/rust#119365 [10]
Link: rust-lang/rust#119364 [11]
Link: https://lore.kernel.org/rust-for-linux/ZWipTZysC2YL7qsq@Boquns-Mac-mini.home/ [12]
Link: rust-lang/rust#120932 [13]
Link: rust-lang/rust#120969 [14]
Link: rust-lang/rust#121202 [15]
Link: rust-lang/rust#121237 [16]
Signed-off-by: Miguel Ojeda <ojeda@kernel.org>
ojeda added a commit to ojeda/linux that referenced this pull request Apr 1, 2024
This is the next upgrade to the Rust toolchain, from 1.77.1 to 1.78.0
(i.e. the latest) [1].

See the upgrade policy [2] and the comments on the first upgrade in
commit 3ed03f4 ("rust: upgrade to Rust 1.68.2").

# Unstable features

There have been no changes to the set of unstable features used in
our own code. Therefore, the only unstable features allowed to be used
outside the `kernel` crate is still `new_uninit`.

However, since we are finally dropping our `alloc` fork [3], all the
unstable features used by `alloc` (~30 language ones, ~60 library ones)
are not a concern anymore. This reduces the maintenance burden, increases
the chances of new compiler versions working without changes and gets
us closer to the goal of supporting several compiler versions.

It also means that, ignoring non-language/library features, we are
currently left with just the few language features needed to implement the
kernel `Arc`, the `new_uninit` library feature, the `compiler_builtins`
marker and the few `no_*` `cfg`s we pass when compiling `core`/`alloc`.

Please see [4] for details.

# Required changes

## LLVM's data layout

Rust 1.77.0 (i.e. the previous upgrade) introduced a check for matching
LLVM data layouts [5]. Then, Rust 1.78.0 upgraded LLVM's bundled major
version from 17 to 18 [6], which changed the data layout in x86 [7]. Thus
update the data layout in our custom target specification for x86 so
that the compiler does not complain about the mismatch:

    error: data-layout for target `target-5559158138856098584`,
    `e-m:e-p270:32:32-p271:32:32-p272:64:64-i64:64-f80:128-n8:16:32:64-S128`,
    differs from LLVM target's `x86_64-linux-gnu` default layout,
    `e-m:e-p270:32:32-p271:32:32-p272:64:64-i64:64-i128:128-f80:128-n8:16:32:64-S128`

In the future, the goal is to drop the custom target specifications.
Meanwhile, if we want to support other LLVM versions used in `rustc`
(e.g. for LTO), we will need to add some extra logic (e.g. conditional on
LLVM's version, or extracting the data layout from an existing built-in
target specification).

## `unused_imports`

Rust's `unused_imports` lint covers both unused and redundant imports.
Now, in 1.78.0, the lint detects more cases of redundant imports [8].
Thus one of the previous patches cleaned them up.

## Clippy's `new_without_default`

Clippy now suggests to implement `Default` even when `new()` is `const`,
since `Default::default()` may call `const` functions even if it is not
`const` itself [9]. Thus one of the previous patches implemented it.

# Other changes in Rust

Rust 1.78.0 introduced `feature(asm_goto)` [10] [11]. This feature was
discussed in the past [12].

Rust 1.78.0 introduced support for mutable pointers to Rust statics,
including a test case for the Linux kernel's `VTABLE` use case [13].

Rust 1.78.0 with debug assertions enabled (i.e. `-Cdebug-assertions=y`,
kernel's `CONFIG_RUST_DEBUG_ASSERTIONS=y`) now always checks all unsafe
preconditions, without a way to opt-out for particular cases [14].

Rust 1.78.0 also improved a couple issues we reported when giving feedback
for the new `--check-cfg` feature [15] [16].

# `alloc` upgrade and reviewing

As mentioned above, compiler upgrades will not update `alloc` anymore,
since we are dropping our `alloc` fork [3].

Link: https://github.com/rust-lang/rust/blob/stable/RELEASES.md#version-1780-2024-05-02 [1]
Link: https://rust-for-linux.com/rust-version-policy [2]
Link: https://lore.kernel.org/rust-for-linux/20240328013603.206764-1-wedsonaf@gmail.com/ [3]
Link: Rust-for-Linux#2 [4]
Link: rust-lang/rust#120062 [5]
Link: rust-lang/rust#120055 [6]
Link: https://reviews.llvm.org/D86310 [7]
Link: rust-lang/rust#117772 [8]
Link: rust-lang/rust-clippy#10903 [9]
Link: rust-lang/rust#119365 [10]
Link: rust-lang/rust#119364 [11]
Link: https://lore.kernel.org/rust-for-linux/ZWipTZysC2YL7qsq@Boquns-Mac-mini.home/ [12]
Link: rust-lang/rust#120932 [13]
Link: rust-lang/rust#120969 [14]
Link: rust-lang/rust#121202 [15]
Link: rust-lang/rust#121237 [16]
Signed-off-by: Miguel Ojeda <ojeda@kernel.org>
ojeda added a commit to ojeda/linux that referenced this pull request May 5, 2024
This is the next upgrade to the Rust toolchain, from 1.77.1 to 1.78.0
(i.e. the latest) [1].

See the upgrade policy [2] and the comments on the first upgrade in
commit 3ed03f4 ("rust: upgrade to Rust 1.68.2").

It is much smaller than previous upgrades, since the `alloc` fork was
dropped in commit 9d0441b ("rust: alloc: remove our fork of the
`alloc` crate") [3].

# Unstable features

There have been no changes to the set of unstable features used in
our own code. Therefore, the only unstable features allowed to be used
outside the `kernel` crate is still `new_uninit`.

However, since we finally dropped our `alloc` fork [3], all the unstable
features used by `alloc` (~30 language ones, ~60 library ones) are not
a concern anymore. This reduces the maintenance burden, increases the
chances of new compiler versions working without changes and gets us
closer to the goal of supporting several compiler versions.

It also means that, ignoring non-language/library features, we are
currently left with just the few language features needed to implement the
kernel `Arc`, the `new_uninit` library feature, the `compiler_builtins`
marker and the few `no_*` `cfg`s we pass when compiling `core`/`alloc`.

Please see [4] for details.

# Required changes

## LLVM's data layout

Rust 1.77.0 (i.e. the previous upgrade) introduced a check for matching
LLVM data layouts [5]. Then, Rust 1.78.0 upgraded LLVM's bundled major
version from 17 to 18 [6], which changed the data layout in x86 [7]. Thus
update the data layout in our custom target specification for x86 so
that the compiler does not complain about the mismatch:

    error: data-layout for target `target-5559158138856098584`,
    `e-m:e-p270:32:32-p271:32:32-p272:64:64-i64:64-f80:128-n8:16:32:64-S128`,
    differs from LLVM target's `x86_64-linux-gnu` default layout,
    `e-m:e-p270:32:32-p271:32:32-p272:64:64-i64:64-i128:128-f80:128-n8:16:32:64-S128`

In the future, the goal is to drop the custom target specifications.
Meanwhile, if we want to support other LLVM versions used in `rustc`
(e.g. for LTO), we will need to add some extra logic (e.g. conditional on
LLVM's version, or extracting the data layout from an existing built-in
target specification).

## `unused_imports`

Rust's `unused_imports` lint covers both unused and redundant imports.
Now, in 1.78.0, the lint detects more cases of redundant imports [8].
Thus one of the previous patches cleaned them up.

## Clippy's `new_without_default`

Clippy now suggests to implement `Default` even when `new()` is `const`,
since `Default::default()` may call `const` functions even if it is not
`const` itself [9]. Thus one of the previous patches implemented it.

# Other changes in Rust

Rust 1.78.0 introduced `feature(asm_goto)` [10] [11]. This feature was
discussed in the past [12].

Rust 1.78.0 introduced `feature(const_refs_to_static)` [13] to allow
referencing statics in constants and extended `feature(const_mut_refs)`
to allow raw mutable pointers in constants. Together, this should cover
the kernel's `VTABLE` use case. In fact, the implementation [14] in
upstream Rust added a test case for it [15].

Rust 1.78.0 with debug assertions enabled (i.e. `-Cdebug-assertions=y`,
kernel's `CONFIG_RUST_DEBUG_ASSERTIONS=y`) now always checks all unsafe
preconditions, though without a way to opt-out for particular cases [16].
It would be ideal to have a way to selectively disable certain checks
per-call site for this one (i.e. not just per check but for particular
instances of a check), even if the vast majority of the checks remain
in place [17].

Rust 1.78.0 also improved a couple issues we reported when giving feedback
for the new `--check-cfg` feature [18] [19].

# `alloc` upgrade and reviewing

As mentioned above, compiler upgrades will not update `alloc` anymore,
since we dropped our `alloc` fork [3].

Link: https://github.com/rust-lang/rust/blob/stable/RELEASES.md#version-1780-2024-05-02 [1]
Link: https://rust-for-linux.com/rust-version-policy [2]
Link: https://lore.kernel.org/rust-for-linux/20240328013603.206764-1-wedsonaf@gmail.com/ [3]
Link: Rust-for-Linux#2 [4]
Link: rust-lang/rust#120062 [5]
Link: rust-lang/rust#120055 [6]
Link: https://reviews.llvm.org/D86310 [7]
Link: rust-lang/rust#117772 [8]
Link: rust-lang/rust-clippy#10903 [9]
Link: rust-lang/rust#119365 [10]
Link: rust-lang/rust#119364 [11]
Link: https://lore.kernel.org/rust-for-linux/ZWipTZysC2YL7qsq@Boquns-Mac-mini.home/ [12]
Link: rust-lang/rust#119618 [13]
Link: rust-lang/rust#120932 [14]
Link: https://github.com/rust-lang/rust/pull/120932/files#diff-e6fc1622c46054cd46b1d225c5386c5554564b3b0fa8a03c2dc2d8627a1079d9 [15]
Link: rust-lang/rust#120969 [16]
Link: Rust-for-Linux#354 [17]
Link: rust-lang/rust#121202 [18]
Link: rust-lang/rust#121237 [19]
Reviewed-by: Alice Ryhl <aliceryhl@google.com>
Link: https://lore.kernel.org/r/20240401212303.537355-4-ojeda@kernel.org
[ Added a few more details and links I mentioned in the list. - Miguel ]
Signed-off-by: Miguel Ojeda <ojeda@kernel.org>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
F-check-cfg --check-cfg S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants